rules/work/genes/scripts/gnomad_constraints_v4_to_tsv.py (4)

49-52: Use idiomatic Python boolean comparisons

Avoid explicit comparisons to boolean literals.

-discard = df.filter(pl.col("mane_or_canonical") == False).select(
+discard = df.filter(~pl.col("mane_or_canonical")).select(
    ["gene", "gene_id"]
)

🧰 Tools

🪛 Ruff (0.8.2)

49-49: Avoid equality comparisons to False; use if not pl.col("mane_or_canonical"): for false checks

Replace with not pl.col("mane_or_canonical")

(E712)

---

52-52: Consider using a logging mechanism instead of print

For better integration with the Snakemake workflow, consider using a proper logging mechanism instead of print statements.

-print("No mane_select or canonical transcript found for ", discard.select("gene"))
+import logging
+logging.info(f"No mane_select or canonical transcript found for {discard.select('gene')}")

---

54-55: Use idiomatic Python boolean comparisons

Avoid explicit comparisons to boolean literals.

-df = (
-    df.filter(pl.col("mane_or_canonical") == True)
+df = (
+    df.filter(pl.col("mane_or_canonical"))

🧰 Tools

🪛 Ruff (0.8.2)

54-54: Avoid equality comparisons to True; use if pl.col("mane_or_canonical"): for truth checks

Replace with pl.col("mane_or_canonical")

(E712)

---

67-71: Handle undefined snakemake variable for better IDE integration

The snakemake variable is undefined in this file and only available when the script is run through Snakemake. Consider adding a conditional import or check to make the script more IDE-friendly and to address static analysis warnings.

 if __name__ == "__main__":
+    try:
+        # When run by snakemake
         main(
             snakemake.input.tsv,
             snakemake.input.xlink_ensembl,
             snakemake.output.tsv,
             snakemake.output.tsv_md5,
         )
+    except NameError:
+        # For development/testing outside of snakemake
+        import argparse
+        parser = argparse.ArgumentParser()
+        parser.add_argument("--constraint-tsv", required=True)
+        parser.add_argument("--ensembl-xlink-tsv", required=True)
+        parser.add_argument("--output-tsv", required=True)
+        parser.add_argument("--output-tsv-md5", required=True)
+        args = parser.parse_args()
+        main(
+            args.constraint_tsv,
+            args.ensembl_xlink_tsv,
+            args.output_tsv,
+            args.output_tsv_md5
+        )

🧰 Tools

🪛 Ruff (0.8.2)

67-67: Undefined name snakemake

(F821)

---

68-68: Undefined name snakemake

(F821)

---

69-69: Undefined name snakemake

(F821)

---

70-70: Undefined name snakemake

(F821)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

rules/work/genes/scripts/gnomad_constraints_v4_to_tsv.py (5)

36-39: Consider adding error handling for file operations

The file reading operations don't have error handling. If one of the input files doesn't exist or has an unexpected format, the script will crash without a helpful error message.

-df = pl.read_csv(constraint_tsv_path, separator="\t", null_values="NA")
-ensembl_xlink = pl.read_csv(
-    ensembl_xlink_tsv_path, separator="\t", null_values="NA"
-)
+try:
+    df = pl.read_csv(constraint_tsv_path, separator="\t", null_values="NA")
+    ensembl_xlink = pl.read_csv(
+        ensembl_xlink_tsv_path, separator="\t", null_values="NA"
+    )
+except Exception as e:
+    print(f"Error reading input files: {e}")
+    raise

---

49-51: Use Pythonic boolean comparison

Using == False is less Pythonic than using ~ (not) operator or not with Polars expressions.

-discard = df.filter(pl.col("mane_or_canonical") == False).select(
+discard = df.filter(~pl.col("mane_or_canonical")).select(
    ["gene", "gene_id"]
)

🧰 Tools

🪛 Ruff (0.8.2)

49-49: Avoid equality comparisons to False; use if not pl.col("mane_or_canonical"): for false checks

Replace with not pl.col("mane_or_canonical")

(E712)

---

53-55: Use Pythonic boolean comparison

Similar to the previous comment, use the direct boolean expression instead of == True.

-df = (
-    df.filter(pl.col("mane_or_canonical") == True)
+df = (
+    df.filter(pl.col("mane_or_canonical"))

🧰 Tools

🪛 Ruff (0.8.2)

54-54: Avoid equality comparisons to True; use if pl.col("mane_or_canonical"): for truth checks

Replace with pl.col("mane_or_canonical")

(E712)

---

65-71: Add error handling and documentation for Snakemake dependency

The script assumes the snakemake object is available, which is true when run through Snakemake but might cause issues if someone tries to run the script directly. Consider adding a comment or try-except block to handle this dependency.

if __name__ == "__main__":
+    # This script is designed to be run by Snakemake
+    # The snakemake object is automatically available in the namespace
+    try:
        main(
            snakemake.input.tsv,
            snakemake.input.xlink_ensembl,
            snakemake.output.tsv,
            snakemake.output.tsv_md5,
        )
+    except NameError as e:
+        print("This script is designed to be run by Snakemake")
+        print(f"Error: {e}")
+        import sys
+        sys.exit(1)

🧰 Tools

🪛 Ruff (0.8.2)

67-67: Undefined name snakemake

(F821)

---

68-68: Undefined name snakemake

(F821)

---

69-69: Undefined name snakemake

(F821)

---

70-70: Undefined name snakemake

(F821)

---

3-72: Add docstrings for better documentation

The code would benefit from docstrings to explain the purpose of the script and the main function, including its parameters and what it does.

+"""
+Script to convert gnomAD constraints data to TSV format.
+
+This script processes gnomAD constraint data, prioritizing mane_select and 
+canonical transcripts, and converts it to a standardized TSV format.
+"""
import polars as pl

def main(
    constraint_tsv_path, ensembl_xlink_tsv_path, output_tsv_path, output_tsv_md5_path
):
+    """
+    Process gnomAD constraint data and convert it to TSV format.
+    
+    Args:
+        constraint_tsv_path: Path to the input constraint TSV file
+        ensembl_xlink_tsv_path: Path to the Ensembl cross-reference TSV file
+        output_tsv_path: Path to write the output TSV file
+        output_tsv_md5_path: Path to write the MD5 checksum of the output TSV file
+    
+    The function prioritizes mane_select and canonical transcripts when
+    selecting entries for each gene.
+    """

🧰 Tools

🪛 Ruff (0.8.2)

49-49: Avoid equality comparisons to False; use if not pl.col("mane_or_canonical"): for false checks

Replace with not pl.col("mane_or_canonical")

(E712)

---

54-54: Avoid equality comparisons to True; use if pl.col("mane_or_canonical"): for truth checks

Replace with pl.col("mane_or_canonical")

(E712)

---

62-62: Undefined name snakemake

(F821)

---

67-67: Undefined name snakemake

(F821)

---

68-68: Undefined name snakemake

(F821)

---

69-69: Undefined name snakemake

(F821)

---

70-70: Undefined name snakemake

(F821)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: tedil
PR: varfish-org/varfish-db-downloader#101
File: rules/work/genes/scripts/gnomad_constraints_v4_to_tsv.py:62-62
Timestamp: 2025-03-10T08:16:40.589Z
Learning: When writing Python scripts for Snakemake (used with the `script:` directive), the `snakemake` object is automatically available in the script's namespace. You can access Snakemake's shell function using `snakemake.shell()` rather than importing subprocess.

rules/work/genes/scripts/gnomad_constraints_v4_to_tsv.py (1)

50-56: Use Pythonic boolean comparisons

Current boolean comparisons use explicit equality checks with True/False literals, which is not idiomatic Python.

-    discard = df.filter(pl.col("mane_or_canonical") == False).select(
+    discard = df.filter(~pl.col("mane_or_canonical")).select(
     ["gene", "gene_id"]
    )
    print("No mane_select or canonical transcript found for ", discard.select("gene"))
    df = (
-        df.filter(pl.col("mane_or_canonical") == True)
+        df.filter(pl.col("mane_or_canonical"))

🧰 Tools

🪛 Ruff (0.8.2)

50-50: Avoid equality comparisons to False; use if not pl.col("mane_or_canonical"): for false checks

Replace with not pl.col("mane_or_canonical")

(E712)

---

55-55: Avoid equality comparisons to True; use if pl.col("mane_or_canonical"): for truth checks

Replace with pl.col("mane_or_canonical")

(E712)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: tedil
PR: varfish-org/varfish-db-downloader#101
File: rules/work/genes/scripts/gnomad_constraints_v4_to_tsv.py:62-62
Timestamp: 2025-03-10T08:16:40.589Z
Learning: When writing Python scripts for Snakemake (used with the `script:` directive), the `snakemake` object is automatically available in the script's namespace. You can access Snakemake's shell function using `snakemake.shell()` rather than importing subprocess.

rules/work/genes/scripts/gnomad_constraints_v4_to_tsv.py (8)

1-3: Consider adding a docstring to explain script's purpose.

The script lacks a module-level docstring that would help users understand its purpose, inputs, and expected outputs. Since this script is replacing inline functionality from a Snakemake file, clear documentation would be valuable.

 import polars as pl
 from snakemake.shell import shell
 
+"""
+Process gnomAD v4 constraint data prioritizing MANE select and canonical transcripts.
+
+This script converts gnomAD v4 constraint data to the VarFish TSV format, ensuring
+that MANE select and canonical transcripts are prioritized for each gene.
+"""
+

---

4-6: Add a function docstring with parameter descriptions.

Adding a proper docstring to the main function would improve code readability and maintenance.

 def main(
     constraint_tsv_path, ensembl_xlink_tsv_path, output_tsv_path, output_tsv_md5_path
 ):
+    """
+    Convert gnomAD v4 constraint data to VarFish TSV format.
+    
+    Args:
+        constraint_tsv_path: Path to the input gnomAD constraint TSV file
+        ensembl_xlink_tsv_path: Path to the Ensembl cross-reference TSV file
+        output_tsv_path: Path where the output TSV file will be written
+        output_tsv_md5_path: Path where the MD5 checksum of the output file will be written
+    """

---

42-49: Add a comment explaining the transcript selection logic.

The sorting and filtering logic is complex and crucial to understanding how the preferred transcript is selected for each gene. A comment would clarify the intent.

     df = (
+        # Sort to prioritize MANE select and canonical transcripts, then by CDS length
+        # Group by gene and take the last record (highest priority transcript per gene)
         df.sort(by=["gene", "mane_select", "canonical", "cds_length", "transcript"])
         .filter(pl.col("gene_id").str.starts_with("ENSG"))
         .group_by(["gene"])
         .last()
         .sort(["gene", "mane_select", "canonical"])
         .with_columns(mane_or_canonical=pl.col("canonical") | pl.col("mane_select"))
     )

---

50-53: Use is_not() for boolean column comparison.

Instead of comparing a boolean column to False, use the more idiomatic Polars expression with is_not().

-    discard = df.filter(pl.col("mane_or_canonical") == False).select(
+    discard = df.filter(pl.col("mane_or_canonical").is_not()).select(
         ["gene", "gene_id"]
     )
     print("No mane_select or canonical transcript found for ", discard.select("gene"))

🧰 Tools

🪛 Ruff (0.8.2)

50-50: Avoid equality comparisons to False; use if not pl.col("mane_or_canonical"): for false checks

Replace with not pl.col("mane_or_canonical")

(E712)

---

53-53: Consider using logging instead of print.

Using Python's logging module would provide better control over output verbosity and destination, which is especially important in a Snakemake workflow.

+import logging
+
+# At the top of the file
+logging.basicConfig(level=logging.INFO)
+
+# Replace the print statement with
-    print("No mane_select or canonical transcript found for ", discard.select("gene"))
+    logging.info(f"No mane_select or canonical transcript found for {discard.select('gene')}")

---

55-55: Use is_() for boolean column comparison.

Instead of comparing a boolean column to True, use the more idiomatic Polars expression with is_().

     df = (
-        df.filter(pl.col("mane_or_canonical") == True)
+        df.filter(pl.col("mane_or_canonical").is_())
         .rename(columns_src_dst)
         .with_columns(pl.lit(None).alias(col) for col in additional_columns)
         .join(ensembl_xlink, on=["ensembl_transcript_id"])
         .select(columns_dst)
     )

🧰 Tools

🪛 Ruff (0.8.2)

55-55: Avoid equality comparisons to True; use if pl.col("mane_or_canonical"): for truth checks

Replace with pl.col("mane_or_canonical")

(E712)

---

57-57: Consider using list comprehension for clarity.

The current code uses generator expression inside with_columns(). For better readability, consider using a list comprehension.

-        .with_columns(pl.lit(None).alias(col) for col in additional_columns)
+        .with_columns([pl.lit(None).alias(col) for col in additional_columns])

---

1-72: Add error handling for robust execution.

The script lacks error handling which could be helpful for diagnosing issues in the Snakemake workflow. Consider adding try-except blocks around critical operations.

 def main(
     constraint_tsv_path, ensembl_xlink_tsv_path, output_tsv_path, output_tsv_md5_path
 ):
+    try:
         columns_src_dst = {
             # ... existing code ...
         }
         
         # ... existing code ...
         
         df.write_csv(output_tsv_path, separator="\t")
         shell(f"md5sum {output_tsv_path} > {output_tsv_md5_path}")
+    except Exception as e:
+        logging.error(f"Error processing gnomAD constraints: {e}")
+        raise

🧰 Tools

🪛 Ruff (0.8.2)

50-50: Avoid equality comparisons to False; use if not pl.col("mane_or_canonical"): for false checks

Replace with not pl.col("mane_or_canonical")

(E712)

---

55-55: Avoid equality comparisons to True; use if pl.col("mane_or_canonical"): for truth checks

Replace with pl.col("mane_or_canonical")

(E712)

---

67-67: Undefined name snakemake

(F821)

---

68-68: Undefined name snakemake

(F821)

---

69-69: Undefined name snakemake

(F821)

---

70-70: Undefined name snakemake

(F821)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: tedil
PR: varfish-org/varfish-db-downloader#101
File: rules/work/genes/scripts/gnomad_constraints_v4_to_tsv.py:62-62
Timestamp: 2025-03-10T08:16:40.589Z
Learning: When writing Python scripts for Snakemake (used with the `script:` directive), the `snakemake` object is automatically available in the script's namespace. You can access Snakemake's shell function using `snakemake.shell()` rather than importing subprocess.

rules/work/genes/scripts/gnomad_constraints_v4_to_tsv.py (5)

33-33: Consider adding a comment explaining the slice operation

The code skips the first value from columns_src_dst.values() (which is "ensembl_transcript_id"). This is intentional since it's included separately in the hardcoded part of the list, but this might not be immediately obvious to other developers.

-        + list(columns_src_dst.values())[1:]
+        # Skip first value "ensembl_transcript_id" as it's already included via the join operation
+        + list(columns_src_dst.values())[1:]

---

50-52: Avoid direct Boolean comparisons in Polars expressions

According to Polars best practices, it's better to avoid equality comparisons to boolean literals.

-    discard = df.filter(pl.col("mane_or_canonical") == False).select(
+    discard = df.filter(~pl.col("mane_or_canonical")).select(

🧰 Tools

🪛 Ruff (0.8.2)

50-50: Avoid equality comparisons to False; use if not pl.col("mane_or_canonical"): for false checks

Replace with not pl.col("mane_or_canonical")

(E712)

---

53-53: Consider using proper logging instead of print

For better integration with Snakemake's logging system, consider using the Python logging module instead of print statements.

-    print("No mane_select or canonical transcript found for ", discard.select("gene"))
+    import logging
+    logging.info(f"No mane_select or canonical transcript found for {discard.select('gene')}")

---

55-55: Avoid direct Boolean comparisons in Polars expressions

Similar to the previous comment, replace the explicit boolean comparison with a more idiomatic expression.

-        df.filter(pl.col("mane_or_canonical") == True)
+        df.filter(pl.col("mane_or_canonical"))

🧰 Tools

🪛 Ruff (0.8.2)

55-55: Avoid equality comparisons to True; use if pl.col("mane_or_canonical"): for truth checks

Replace with pl.col("mane_or_canonical")

(E712)

---

67-73: Add a comment explaining the snakemake object

The snakemake object is automatically available when the script is run through Snakemake's script: directive, but this might not be obvious to all developers.

 if __name__ == "__main__":
+    # The snakemake object is automatically available when this script is run through
+    # Snakemake's script: directive
     main(
         snakemake.input.tsv,
         snakemake.input.xlink_ensembl,
         snakemake.output.tsv,
         snakemake.output.tsv_md5,
     )

🧰 Tools

🪛 Ruff (0.8.2)

69-69: Undefined name snakemake

(F821)

---

70-70: Undefined name snakemake

(F821)

---

71-71: Undefined name snakemake

(F821)

---

72-72: Undefined name snakemake

(F821)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: tedil
PR: varfish-org/varfish-db-downloader#101
File: rules/work/genes/scripts/gnomad_constraints_v4_to_tsv.py:62-62
Timestamp: 2025-03-10T08:16:40.589Z
Learning: When writing Python scripts for Snakemake (used with the `script:` directive), the `snakemake` object is automatically available in the script's namespace. You can access Snakemake's shell function using `snakemake.shell()` rather than importing subprocess.

rules/work/genes/scripts/gnomad_constraints_v4_to_tsv.py (1)

66-67: Duplicate if __name__ == "__main__" check

There are two identical if __name__ == "__main__" checks at lines 66 and 67. The first one is followed by an empty line, which suggests it might be a mistake.

-if __name__ == "__main__":
-
 if __name__ == "__main__":
     main(

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: tedil
PR: varfish-org/varfish-db-downloader#101
File: rules/work/genes/scripts/gnomad_constraints_v4_to_tsv.py:62-62
Timestamp: 2025-03-10T08:16:40.589Z
Learning: When writing Python scripts for Snakemake (used with the `script:` directive), the `snakemake` object is automatically available in the script's namespace. You can access Snakemake's shell function using `snakemake.shell()` rather than importing subprocess.